Skip to content

fix: DESCRIBE PAGE recurses into ScrollContainer / TabControl children#271

Merged
ako merged 1 commit intomendixlabs:mainfrom
hjotha:fix/219-scrollcontainer-tabcontrol-parsing
Apr 23, 2026
Merged

fix: DESCRIBE PAGE recurses into ScrollContainer / TabControl children#271
ako merged 1 commit intomendixlabs:mainfrom
hjotha:fix/219-scrollcontainer-tabcontrol-parsing

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented Apr 23, 2026

Summary

DESCRIBE PAGE walks the widget tree by recursing into Widgets[] on every container, but two widget types store their children elsewhere:

  • ScrollContainer holds children under CenterRegion.Widgets (Mendix 9+). A fallback to the top-level Widgets is kept for older BSON shapes.
  • TabControl holds children under TabPages[].Widgets. The parser now emits a synthetic Pages$TabPage intermediate widget per tab, with the tab name and caption preserved, so the DESCRIBE output distinguishes which tab each nested widget belongs to.

Before this fix every widget nested inside those two containers was invisible in DESCRIBE PAGE output, creating silent gaps for any downstream tooling that relied on DESCRIBE to build a widget-ID-to-page map.

Relationship to #219

This PR is a reimplementation of #219 against the current main. Since #219 was opened the parseRawWidget / outputWidgetMDLV3 signatures were refactored ((e *Executor) method → free function taking *ExecContext), and tabcontainer / tabpage keywords landed in the MDL grammar. Changes vs #219:

  • Updated to the new free-function + *ExecContext API.
  • Lowercase tabcontainer / tabpage / scrollcontainer output to match the current MDL convention.
  • Preserves Pages$TabPage as an intermediate node with tab name and caption (Update parsing of ScrollContainer and TabControl #219 collapsed tab structure, losing the grouping — flagged by @ako in the Update parsing of ScrollContainer and TabControl #219 review).
  • tabcontainer / tabpage already exist in the grammar, so the DESCRIBE output round-trips through mxcli exec. scrollcontainer has no grammar keyword yet; this PR outputs it for DESCRIBE but does not make it re-executable. Grammar addition is a clean follow-up.
  • Added unit tests and an mdl-examples/bug-tests/ reproducer.

Credit to @NikolaSimsic for identifying the original bug in #219 and for the CenterRegion.Widgets / TabPages[].Widgets field insight. Once this merges, #219 can be closed.

Test plan

  • Unit tests in mdl/executor/cmd_pages_describe_container_test.go:
    • parseRawWidget: ScrollContainer via CenterRegion, ScrollContainer legacy-Widgets fallback, TabControl with multiple TabPages (names + child widgets preserved per tab).
    • outputWidgetMDLV3: scroll container header, tab container + tab page + caption emission.
  • Bug-test MDL script at mdl-examples/bug-tests/219-scrollcontainer-tabcontrol-describe.mdl round-trips through mxcli check.
  • go build ./... && go test ./... pass.

@hjothamendix
fix: DESCRIBE PAGE recurses into ScrollContainer and TabControl children
93c84de 
parseRawWidget walked children via the Widgets[] array on every
container, but two widget types store their children elsewhere:

- ScrollContainer holds children under CenterRegion.Widgets (Mendix 9+).
  A fallback to the top-level Widgets remains for older BSON shapes.
- TabControl holds children under TabPages[].Widgets. The parser now
  emits a synthetic Pages$TabPage intermediate widget per tab, with
  the tab name and caption preserved, so DESCRIBE output distinguishes
  which tab each nested widget belongs to.

Before this fix every widget nested inside those two containers was
invisible in DESCRIBE PAGE output, creating silent gaps for any
downstream tooling (e.g. Python-based widget ID extractors used by
test-impact analysis).

Closes hjotha's review of mendixlabs#219 (same idea, rewritten against current
main after the `parseRawWidget`/`outputWidgetMDLV3` signature refactor;
`tabcontainer`/`tabpage` already exist in the grammar, so the
describe output round-trips back through `mxcli exec`). `scrollcontainer`
has no grammar keyword yet — it is output-only for now; grammar addition
is a follow-up.

Credit to @NikolaSimsic for identifying the original bug in mendixlabs#219 and
for the CenterRegion.Widgets / TabPages[].Widgets field insight.

Tests
- parseRawWidget: ScrollContainer with CenterRegion, ScrollContainer
  legacy-Widgets fallback, TabControl with multiple TabPages (name and
  child widgets preserved per tab).
- outputWidgetMDLV3: ScrollContainer header, TabControl + TabPage +
  Caption emission.
- Bug-test MDL script at `mdl-examples/bug-tests/219-scrollcontainer-tabcontrol-describe.mdl`
  round-trips through `mxcli check`.
@hjotha hjotha mentioned this pull request Apr 23, 2026
Closed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

AI Code Review

What Looks Good

  • The PR correctly addresses the bug where DESCRIBE PAGE missed widgets inside ScrollContainer and TabControl containers by modifying the widget tree traversal logic.
  • Comprehensive unit tests cover both container types, including legacy BSON fallbacks and synthetic intermediate widget generation for TabControl.
  • The MDL bug-test script in mdl-examples/bug-tests/ provides a reproducible case that round-trips through mxcli check.
  • Changes follow existing patterns in the codebase (consistent with other container widget handling).
  • The fix preserves tab structure via synthetic Pages$TabPage intermediates as requested in the previous review (Update parsing of ScrollContainer and TabControl #219), improving DESCRIBE output clarity.
  • No silent side effects or error handling gaps introduced; defensive type assertions used appropriately.
  • Scope is tightly focused on the single bug (no unrelated changes).

Recommendation

Approve the PR. The fix is correct, well-tested, and maintains backward compatibility while resolving the silent gaps in DESCRIBE PAGE output for ScrollContainer and TabControl containers. The planned grammar addition for scrollcontainer (to make it re-executable) is appropriately deferred as a follow-up.

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

hjotha pushed a commit to hjotha/mxcli that referenced this pull request Apr 23, 2026
@hjothamendix
merge: fix/219-scrollcontainer-tabcontrol-parsing (PR mendixlabs#271)
7769982
@github-actions github-actions Bot mentioned this pull request Apr 23, 2026
Closed
@ako
Copy link
Copy Markdown
Collaborator

ako commented Apr 23, 2026

PR #271 Review — fix: DESCRIBE PAGE recurses into ScrollContainer / TabControl children

Overview

Clean, focused fix for the silent widget-tree gap in DESCRIBE PAGE. CenterRegion.Widgets path for ScrollContainer and TabPages[].Widgets iteration for TabControl are both correctly added, with the legacy Widgets fallback preserved. Synthetic Pages$TabPage wrappers preserve tab grouping in the output.

What's Good

  • inheritedCtx is correctly defined before the new early-return blocks (lines 27–29 in parseRawWidget) — no scoping issue
  • CaptionTemplate → extractTextCaption fallback chain for tab captions is thorough
  • tabpage Name (Caption: '...') output round-trips correctly through the existing grammar: TABPAGE is in widgetTypeV3, CAPTION COLON stringExprV3 is in widgetPropertyV3, children go in widgetBodyV3
  • scrollcontainer non-roundtrippability is clearly acknowledged — the grammar keyword omission is the right follow-up, not a blocker here
  • 5 focused unit tests cover both parse paths (CenterRegion + legacy fallback, TabControl multi-page) and both output paths (scroll header, tab container structure)

Minor Notes

Appearance extraction duplicated between ScrollContainer and TabControl handlers
Both handlers copy the same Appearance.Class / Appearance.Style / DesignProperties / extractConditionalSettings block. Worth extracting into a small helper like populateWidgetAppearance(w *rawWidget, raw map[string]any) so the pattern stays in one place. Not a blocker.

outputWidgetMDLV3 ScrollContainer and TabControl cases are identical
Both emit header, appendAppearanceProps, recurse children — same code, different case labels. Could share a helper. Again minor.

Tab page appearance properties not extracted
The synthetic Pages$TabPage captures Name, CaptionTemplate, and children but doesn't extract Appearance.Class/Style or DesignProperties from the tab page BSON. Real tab pages do carry these. Not visible in the bug case but worth a follow-up.

Verdict

LGTM. The core fix is correct and well-tested. The minor duplication items are polish, not correctness issues.

@ako ako merged commit a2bc5b3 into mendixlabs:main Apr 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hjotha @ako @hjothamendix